Skip to content

check, knitr - Add .exe to binary on Windows for correct check #12887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 5, 2025
Merged

check, knitr - Add .exe to binary on Windows for correct check #12887

merged 1 commit into from
Jun 5, 2025

Conversation

@cderv
Copy link
Collaborator

@cderv cderv commented Jun 4, 2025

This is a bad problem when you run quarto CLI from another process having R_HOME set (like running from an R session): another R version would be used;

This solves quarto-dev/quarto-r#204, but also problem on Windows

❯ $env:R_HOME="C:/Program Files/R/R-4.5.0"

❯ quarto check knitr
Quarto 99.9.9
[>] Checking R installation...........OK
      Version: 4.3.2
      Path: C:/PROGRA~1/R/R-43~1.2
      LibPaths:
        - C:/Users/chris/AppData/Local/R/win-library/4.3
        - C:/Program Files/R/R-4.3.2/library
      knitr: (None)
      rmarkdown: (None)

      The knitr package is not available in this R installation.
      Install with install.packages("knitr")
      The rmarkdown package is not available in this R installation.
      Install with install.packages("rmarkdown")

This is because we append bin and Rscript and test the path

quarto-cli/src/core/resources.ts

Lines 116 to 121 in 84c6cba

// if there is an R_HOME then respect that
const rHome = Deno.env.get("R_HOME");
debug(`Looking for '${binary}' in R_HOME: ${rHome}`);
if (rHome) {
let rHomeBin = join(rHome, "bin", binary);
if (safeExistsSync(rHomeBin)) {

We need to add .exe on Windows if we test path existence.

Note: I really would like to backport this as this is a bad issue.

cderv added a commit to quarto-dev/quarto-r that referenced this pull request Jun 4, 2025
@cderv
Set QUARTO_R for ealier versions of Quarto to overcome the R_HOME d…
4e67f0a 
…etection problem

Fixed in 1.8 at quarto-dev/quarto-cli#12887
This was referenced Jun 4, 2025
Merged
Closed
@posit-snyk-bot
Copy link
Collaborator

posit-snyk-bot commented Jun 4, 2025 •
edited
Loading

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

✅ license/snyk check is complete. No issues have been found. (View Details)

@cderv cderv merged commit 4c44c81 into main Jun 5, 2025
49 checks passed
@cderv cderv deleted the windows/rscript-detection branch June 5, 2025 08:34
cderv added a commit to quarto-dev/quarto-r that referenced this pull request Jun 5, 2025
@cderv
Correctly use same R version when calling quarto CLI from R (#250)
7f84ed6 
* move import where it is used

* processx can return different error than the one from `error_on_status`

* Set `QUARTO_R` for ealier versions of Quarto to overcome the R_HOME detection problem

Fixed in 1.8 at quarto-dev/quarto-cli#12887

* missing test_path in tests

* Add NEWS

* bump version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cderv @posit-snyk-bot